Skip to content

Feature/1793 - Quarterly SMS reminder to connect with case_contact_types#3580

Merged
harsohailB merged 27 commits into
rubyforgood:mainfrom
harsohailB:feature/1793
Jun 13, 2022
Merged

Feature/1793 - Quarterly SMS reminder to connect with case_contact_types#3580
harsohailB merged 27 commits into
rubyforgood:mainfrom
harsohailB:feature/1793

Conversation

@harsohailB
Copy link
Copy Markdown
Collaborator

@harsohailB harsohailB commented May 29, 2022 •
edited
Loading

What github issue is this PR for, if any?

Resolves #1793

What changed, and why?

  • Created send_case_contact_types_reminder.rake task which runs once per day to check if any volunteers are to be sent an SMS reminder regarding their uncontacted contact types
    • This task will run once per day to check which volunteer should be reminded and send out messages accordingly
  • Created CaseContactTypesReminder class which handles the logic for filtering volunteers that need to be reminded and uses the TwilioService to send the SMSs

@compwron Note: SHORT_IO_DOMAIN, SHORT_IO_API_KEY, BASE_URLenvironment variables will need to be added toconfig/credentials/production.yml.enc` for short io service to work correctly after this PR

TODO

  • Add link to new case contact page in SMS (currently for some reason Twilio blocks the message with the link)
  • Store ShortIO API key in environment variables
  • Figure out application.js error
  • Write pending tests for UserCaseContactTypesReminder model

How will this affect user permissions?

  • Volunteer permissions: N/A
  • Supervisor permissions: N/A
  • Admin permissions: N/A

How is this tested? (please write tests!) 💖💪

  • Wrote tests with various conditions to test if SMS reminder is sent or not (if user has sms notifications on/off, if user has uncontacted contact types from 60 or more days ago, if user has a phone numbers, etc.)

Screenshots please :)

Note: The "Sent from Twilio trial account" will not be shown in production.

@harsohailB harsohailB added ruby Touches Ruby code zzz_archived: 📱 SMS work relating to SMS notifications #1017 codethechange code.the.change developers labels May 29, 2022
@harsohailB harsohailB requested review from 7riumph and xihai01 May 29, 2022 20:31
@harsohailB harsohailB self-assigned this May 29, 2022
harsohailB
harsohailB commented May 29, 2022
View reviewed changes
Comment thread lib/tasks/case_contact_types_reminder.rb Outdated
xihai01
xihai01 reviewed May 29, 2022
View reviewed changes
Comment thread lib/tasks/case_contact_types_reminder.rb Outdated
@harsohailB harsohailB changed the title Feature/1793 Feature/1793 - Quarterly SMS reminder to connect with case_contact_types May 29, 2022
xihai01
xihai01 reviewed May 29, 2022
View reviewed changes
Comment thread lib/tasks/case_contact_types_reminder.rb
xihai01
xihai01 reviewed May 29, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@xihai01 xihai01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. Just need to find a way to send the link now

@harsohailB
Copy link
Copy Markdown
Collaborator Author

harsohailB commented May 29, 2022 •
edited
Loading

lgtm. Just need to find a way to send the link now

Also need to figure out how to make stubbed requests for all user agents. My dev env uses:
"User-Agent" => "twilio-ruby/5.67.1 (darwin21 x86_64) Ruby/3.1.0"
whereas CI uses:
"User-Agent" => "twilio-ruby/5.67.1 (linux x86_64) Ruby/3.1.0" and others

Weird that the stubs for twilio service testing did not require a user agent...

@xihai01
Copy link
Copy Markdown
Collaborator

xihai01 commented May 29, 2022 •
edited
Loading

lgtm. Just need to find a way to send the link now

Also need to figure out how to make stubbed requests for all user agents. My dev env uses: "User-Agent" => "twilio-ruby/5.67.1 (darwin21 x86_64) Ruby/3.1.0" whereas CI uses: "User-Agent" => "twilio-ruby/5.67.1 (linux x86_64) Ruby/3.1.0" and others

Weird that the stubs for twilio service testing did not require a user agent...

I don't think you need to include those in the headers. I just included the important stuff that Twilio needs like content-type and authorization.

@compwron
Copy link
Copy Markdown
Collaborator

compwron commented May 30, 2022

Very cool! I see this is a draft, do you want review yet?

@harsohailB
Copy link
Copy Markdown
Collaborator Author

harsohailB commented May 30, 2022 •
edited
Loading

Very cool! I see this is a draft, do you want review yet?

Currently working on two things:

  • Add link to new case contact page from SMS
  • Figure out short link service api key params storage
  • Write pending tests for UserCaseContactTypesReminder model

Once these are completed, I will change the PR for review!

compwron
compwron reviewed Jun 10, 2022
View reviewed changes
Comment thread lib/tasks/case_contact_types_reminder.rb Outdated
compwron
compwron reviewed Jun 10, 2022
View reviewed changes
Comment thread lib/tasks/send_case_contact_types_reminder.rake Outdated
compwron
compwron reviewed Jun 10, 2022
View reviewed changes
Comment thread spec/support/webmock_helper.rb Outdated
compwron
compwron reviewed Jun 10, 2022
View reviewed changes
Comment thread spec/support/webmock_helper.rb Outdated
compwron
compwron requested changes Jun 10, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@compwron compwron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See requested changes

@harsohailB harsohailB marked this pull request as ready for review June 12, 2022 05:46
@harsohailB harsohailB requested review from compwron and xihai01 June 12, 2022 17:53
xihai01
xihai01 reviewed Jun 12, 2022
View reviewed changes
Comment thread lib/tasks/case_contact_types_reminder.rb Outdated
xihai01
xihai01 reviewed Jun 12, 2022
View reviewed changes
Comment thread spec/lib/tasks/case_contact_types_reminder_spec.rb
require_relative "../../../lib/tasks/case_contact_types_reminder"
require "support/webmock_helper"

RSpec.describe CaseContactTypesReminder do
Copy link
Copy Markdown
Collaborator

@xihai01 xihai01 Jun 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i think the tests here can be organized better to avoid too much duplication. To me, I feel the contexts are being crammed with too many cases.

xihai01
xihai01 reviewed Jun 12, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@xihai01 xihai01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

compwron
compwron approved these changes Jun 13, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@compwron compwron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very cool! merge when green

@harsohailB
Copy link
Copy Markdown
Collaborator Author

harsohailB commented Jun 13, 2022

very cool! merge when green

will do after addressing @xihai01 comments!

@harsohailB harsohailB merged commit d9cd04f into rubyforgood:main Jun 13, 2022
7riumph
7riumph reviewed Jun 13, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@7riumph 7riumph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, this is great and a milestone too! It'll be our first SMS deliverable in production 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@7riumph 7riumph 7riumph left review comments

@xihai01 xihai01 xihai01 left review comments

@compwron compwron compwron approved these changes

Assignees

@harsohailB harsohailB

Labels

codethechange code.the.change developers ruby Touches Ruby code 🧪 Tests Tests zzz_archived: 📱 SMS work relating to SMS notifications #1017

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

quarterly SMS reminder to connect with case_contact_types

4 participants

@harsohailB @xihai01 @compwron @7riumph